Get ready for physics benchmarks - #37
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new reconstruction benchmarking utility under analysis/benchmarks that extracts a compact, machine-readable JSON summary from EDM4hep RECO ROOT files, intended for automated comparisons (e.g., CI artifacts and release-to-release validation).
Changes:
- Added
extract_metrics.py, a ROOT-based extractor that summarizes truth, PFO matching, track occupancy, and calorimeter hit/energy metrics into a compact JSON schema. - Added ROOT-independent unit tests for helper functions used by the extractor.
- Added documentation for running the extractor and positioned the benchmarks tooling within the analysis directory structure.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| analysis/README.md | Mentions the new benchmarks/ directory and its purpose. |
| analysis/benchmarks/README.md | Documents extractor behavior, matching definition, and how to run tests. |
| analysis/benchmarks/extract_metrics.py | Implements JSON metrics extraction from EDM4hep RECO files using PyROOT/RDataFrame + JIT helpers. |
| analysis/benchmarks/test_extract_metrics.py | Adds ROOT-independent unit tests for parsing and stats/provenance helper functions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This is an example output of running this on a pion gun sample with 100 events: |
| "path": str(pathlib.Path(__file__).resolve()), | ||
| "sha256": sha256(pathlib.Path(__file__).resolve()), | ||
| }, | ||
| "software": {"root_version": str(ROOT.gROOT.GetVersion())}, |
There was a problem hiding this comment.
Can we make this extract the mucoll-stack version instead of ROOT?
There was a problem hiding this comment.
Yes, that's a good idea.
|
I am wondering a little bit about the metric extraction strategy: would it be cleaner to have a central helper for the deltaR matching, and actually compute all metrics in the various "study_muons.py" etc, so that each independently produces and object-specific json and the script here just aggregates whatever is available at the end? |
|
Another point for discussion: I think are are reasons to wire this up both in MAIAConfig (etc) and at the end of the physval in mucoll-spack. Is this what you were planning to do? |
This PR adds a compact JSON metrics extractor for automated reconstruction validation.
The extractor reads one or more EDM4hep RECO files and records:
This PR intentionally does not define pass/fail tolerances. The initial metrics are report-only so that their statistical viability can be reviewed in the context of the CI worker limits.
Testing
Follow-up
A subsequent
mucoll-spackPR will run this code as part of a small GitHub Actions validation test, but that PR won't work until this one is merged. I can open it in advance if it's easier to review them together...